Skip to content

Fix - #68

Merged
Tyjfre-j merged 41 commits into
developfrom
Fix
Jul 28, 2026
Merged

Fix#68
Tyjfre-j merged 41 commits into
developfrom
Fix

Conversation

@Tyjfre-j

Copy link
Copy Markdown
Collaborator

Summary

Replaces the original Python-side session cap enforcement with a database-native eviction strategy using FOR UPDATE SKIP LOCKED, eliminating race conditions under concurrent logins without adding application-level locking.

What changed

SQL queries (db/queries/session.sql)

Added:

  • EvictOverflowSessionsDELETE ... RETURNING with FOR UPDATE SKIP LOCKED subquery. Computes overflow internally via COUNT(*), sorts by (last_active, created_at), and atomically evicts the oldest sessions.
  • LockUserSessions — advisory SELECT ... FOR UPDATE to serialize per-user session mutations.

Removed:

  • EvictOldestSessions — replaced by EvictOverflowSessions (different parameter contract: passes session_limit instead of pre-computed limit).

Service layer (app/service/users.py)

_create_mobile_session flow updated:

  1. lock_user_sessions — serialize per-user mutations (acquires row lock on all user sessions)
  2. _ensure_device_for_login — get/create device row
  3. upsert_session — create/replace session for this device
  4. evict_overflow_sessions — evict oldest if over cap (yields evicted session IDs)
  5. Redis cache eviction + logging for each evicted session

Replaces Python-side list_sessions_by_user + in-memory sort + delete_session_by_id with atomic SQL eviction.

Test updates

Updated fixtures/fakes across 4 test files to match new query signatures:

  • tests/unit/test_auth_service.pyTestSessionLimit updated for evict_overflow_sessions + lock_user_sessions; added test_multiple_new_devices_at_cap_evict_exact_overflow
  • tests/unit/test_mobile_auth_intent_validation.py — stateful FakeSessionQuerier now computes overflow internally from session_limit
  • tests/unit/test_mobile_auth_email_logging.py — added missing lock_user_sessions + evict_overflow_sessions
  • tests/unit/test_auth_email_otp.py — configured evict_overflow_sessions on bare AsyncMock (async generator side-effect)

Added integration test:

  • test_concurrent_new_device_logins_settle_at_cap_real_db — stress test using separate DB connections per concurrent task; validates final session count == SESSION_LIMIT under true Postgres concurrency

Why SKIP LOCKED

Without it, two concurrent logins from the same user at cap could both read "3 sessions," both decide to evict, and both create a new session → 4 sessions briefly exist. FOR UPDATE SKIP LOCKED makes the subquery rows invisible to competing transactions, so evictions serialize cleanly at the database level.

The advisory lock (lock_user_sessions) ensures the entire device-lookup → upsert → evict sequence is serialized per user, preventing interleaving between upsert_session and evict_overflow_sessions.

@Tyjfre-j
Tyjfre-j marked this pull request as draft July 20, 2026 12:16
Tyjfre-j added 19 commits July 24, 2026 02:13
@Tyjfre-j

Copy link
Copy Markdown
Collaborator Author

Changes

Security fixes

  • grace-window cache-hit replay now re-checks blocked status against
    Postgres before returning cached tokens
  • Fixed a silent fall-through bug: a used refresh token within its grace window
    with no cached replay available previously fell through into a fresh
    rotation off an already-used token row. Now rejects outright.
  • Refresh-token replay cache (refresh_retry:{hash} in Redis) is now
    AES-256-GCM encrypted before storage — was previously plaintext JSON,
    including the raw newly-issued refresh token
  • check_rate_limit now fails open on Redis errors instead of raising,
    matching the existing RateLimiter dependency's behavior — a Redis outage
    previously took down login/register/OTP-resend entirely
  • block_user/delete_user now take get_user_by_id_for_update before
    mutating, closing a check-then-act race against mobile_login where a
    concurrent login could create a session invisible to cleanup

Refactor

  • refresh_token split into refresh_token + _handle_used_refresh_token,
    resolving a ruff C901 complexity warning; behavior preserved except for
    the fall-through fix noted above

@Tyjfre-j

Copy link
Copy Markdown
Collaborator Author

Session expiry: split into idle + absolute timeouts

Replaces the single expires_at column with idle_expires_at (slides on activity) and absolute_expires_at (hard ceiling set at login).

What changed

  • DB: added idle_expires_at / absolute_expires_at to user_sessions; removed expires_at.
  • Queries: upsert_session, update_session_activity, get_session_by_id, etc. updated.
  • Auth service: login sets both timers; refresh & validate reject if either is past.
  • Token auth dependency: fast-path Redis check validates both fields; idle timer slides forward on throttled activity (capped at absolute).
  • Response schema: SessionSchema returns both fields.
  • Tests: updated all call sites + added behavior tests for idle-only, absolute-only, and cap scenarios.
  • Migrations: Alembic + raw SQL up/down included.

@Tyjfre-j
Tyjfre-j marked this pull request as ready for review July 28, 2026 17:40
@Tyjfre-j
Tyjfre-j merged commit b753d45 into develop Jul 28, 2026
2 checks passed
@Tyjfre-j
Tyjfre-j deleted the Fix branch July 28, 2026 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant